[clang] Implement provisional wording for CWG2398 regarding packs #90820

mizvekov · 2024-05-02T05:11:38Z

This solves some ambuguity introduced in P0522 regarding how
template template parameters are partially ordered, and should reduce
the negative impact of enabling -frelaxed-template-template-args
by default.

When performing template argument deduction, a template template parameter
containing no packs should be more specialized than one that does.

Given the following example:

template<class T2> struct A;
template<template<class ...T3s> class TT1, class T4> struct A<TT1<T4>>; // #1
template<template<class    T5 > class TT2, class T6> struct A<TT2<T6>>; // #2

template<class T1> struct B;
template struct A<B<char>>;

Prior to P0522, candidate #2 would be more specialized.
After P0522, neither is more specialized, so this becomes ambiguous.
With this change, #2 becomes more specialized again,
maintaining compatibility with pre-P0522 implementations.

The problem is that in P0522, candidates are at least as specialized
when matching packs to fixed-size lists both ways, whereas before,
a fixed-size list is more specialized.

This patch keeps the original behavior when checking template arguments
outside deduction, but restores this aspect of pre-P0522 matching
during deduction.

Since this changes provisional implementation of CWG2398 which has
not been released yet, and already contains a changelog entry,
we don't provide a changelog entry here.

llvmbot · 2024-05-02T05:12:08Z

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling -frelaxed-template-template-args by default.

A template template parameter containing no packs should be more specialized than one that does.

Given the following example:

template&lt;class T2&gt; struct A;
template&lt;template&lt;class ...T3s&gt; class TT1, class T4&gt; struct A&lt;TT1&lt;T4&gt;&gt;; #<!-- -->1
template&lt;template&lt;class    T5 &gt; class TT2, class T6&gt; struct A&lt;TT2&lt;T6&gt;&gt;; #<!-- -->2

template&lt;class T1&gt; struct B;
template struct A&lt;B&lt;char&gt;&gt;;

Prior to P0522, #2 would be the only viable candidate. After P0522, #1 is also viable, and neither is considered more specialized, so this becomes ambiguous.
With this change, #2 is considered more specialized, so that is what is picked and we remain compatible with pre-P0522 implementations.

The problem we solve here is that the specialization rules in [temp.arg.template]/4 are defined in terms of a rewrite to function templates, but in the case of partial ordering, the rules on how P and A lists are matched to each other is swapped regarding how specialization makes sense conceptually.

Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.

Full diff: https://github.com/llvm/llvm-project/pull/90820.diff

4 Files Affected:

(modified) clang/include/clang/Sema/Sema.h (+3-2)
(modified) clang/lib/Sema/SemaTemplate.cpp (+6-4)
(modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+59-20)
(modified) clang/test/SemaTemplate/cwg2398.cpp (-6)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a80ac6dbc76137..2bea8732f61fcd 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9238,7 +9238,7 @@ class Sema final : public SemaBase {
                                    CheckTemplateArgumentKind CTAK);
   bool CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
                                      TemplateParameterList *Params,
-                                     TemplateArgumentLoc &Arg);
+                                     TemplateArgumentLoc &Arg, bool IsDeduced);
 
   void NoteTemplateLocation(const NamedDecl &Decl,
                             std::optional<SourceRange> ParamRange = {});
@@ -9708,7 +9708,8 @@ class Sema final : public SemaBase {
                                     sema::TemplateDeductionInfo &Info);
 
   bool isTemplateTemplateParameterAtLeastAsSpecializedAs(
-      TemplateParameterList *PParam, TemplateDecl *AArg, SourceLocation Loc);
+      TemplateParameterList *PParam, TemplateDecl *AArg, SourceLocation Loc,
+      bool IsDeduced);
 
   void MarkUsedTemplateParameters(const Expr *E, bool OnlyDeduced,
                                   unsigned Depth, llvm::SmallBitVector &Used);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 989f3995ca5991..2c921d4365db0a 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -6384,7 +6384,8 @@ bool Sema::CheckTemplateArgument(
 
   case TemplateArgument::Template:
   case TemplateArgument::TemplateExpansion:
-    if (CheckTemplateTemplateArgument(TempParm, Params, Arg))
+    if (CheckTemplateTemplateArgument(TempParm, Params, Arg,
+                                      /*IsDeduced=*/CTAK != CTAK_Specified))
       return true;
 
     SugaredConverted.push_back(Arg.getArgument());
@@ -8296,7 +8297,8 @@ static void DiagnoseTemplateParameterListArityMismatch(
 /// It returns true if an error occurred, and false otherwise.
 bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
                                          TemplateParameterList *Params,
-                                         TemplateArgumentLoc &Arg) {
+                                         TemplateArgumentLoc &Arg,
+                                         bool IsDeduced) {
   TemplateName Name = Arg.getArgument().getAsTemplateOrTemplatePattern();
   TemplateDecl *Template = Name.getAsTemplateDecl();
   if (!Template) {
@@ -8348,8 +8350,8 @@ bool Sema::CheckTemplateTemplateArgument(TemplateTemplateParmDecl *Param,
         !Template->hasAssociatedConstraints())
       return false;
 
-    if (isTemplateTemplateParameterAtLeastAsSpecializedAs(Params, Template,
-                                                          Arg.getLocation())) {
+    if (isTemplateTemplateParameterAtLeastAsSpecializedAs(
+            Params, Template, Arg.getLocation(), IsDeduced)) {
       // P2113
       // C++20[temp.func.order]p2
       //   [...] If both deductions succeed, the partial ordering selects the
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 9f9e4422827173..4fbaecd7dba960 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -145,7 +145,7 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
                         ArrayRef<TemplateArgument> As,
                         TemplateDeductionInfo &Info,
                         SmallVectorImpl<DeducedTemplateArgument> &Deduced,
-                        bool NumberOfArgumentsMustMatch);
+                        bool NumberOfArgumentsMustMatch, bool Swapped);
 
 static void MarkUsedTemplateParameters(ASTContext &Ctx,
                                        const TemplateArgument &TemplateArg,
@@ -703,9 +703,9 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams,
     // Perform template argument deduction on each template
     // argument. Ignore any missing/extra arguments, since they could be
     // filled in by default arguments.
-    return DeduceTemplateArguments(S, TemplateParams, PResolved,
-                                   SA->template_arguments(), Info, Deduced,
-                                   /*NumberOfArgumentsMustMatch=*/false);
+    return DeduceTemplateArguments(
+        S, TemplateParams, PResolved, SA->template_arguments(), Info, Deduced,
+        /*NumberOfArgumentsMustMatch=*/false, /*Swapped=*/false);
   }
 
   // If the argument type is a class template specialization, we
@@ -731,7 +731,8 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams,
   // Perform template argument deduction for the template arguments.
   return DeduceTemplateArguments(S, TemplateParams, PResolved,
                                  SA->getTemplateArgs().asArray(), Info, Deduced,
-                                 /*NumberOfArgumentsMustMatch=*/true);
+                                 /*NumberOfArgumentsMustMatch=*/true,
+                                 /*Swapped=*/false);
 }
 
 static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
@@ -2552,7 +2553,7 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
                         ArrayRef<TemplateArgument> As,
                         TemplateDeductionInfo &Info,
                         SmallVectorImpl<DeducedTemplateArgument> &Deduced,
-                        bool NumberOfArgumentsMustMatch) {
+                        bool NumberOfArgumentsMustMatch, bool Swapped) {
   // C++0x [temp.deduct.type]p9:
   //   If the template argument list of P contains a pack expansion that is not
   //   the last template argument, the entire template argument list is a
@@ -2583,8 +2584,11 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
         return TemplateDeductionResult::MiscellaneousDeductionFailure;
 
       // Perform deduction for this Pi/Ai pair.
-      if (auto Result = DeduceTemplateArguments(S, TemplateParams, P,
-                                                As[ArgIdx], Info, Deduced);
+      TemplateArgument Pi = P, Ai = As[ArgIdx];
+      if (Swapped)
+        std::swap(Pi, Ai);
+      if (auto Result =
+              DeduceTemplateArguments(S, TemplateParams, Pi, Ai, Info, Deduced);
           Result != TemplateDeductionResult::Success)
         return Result;
 
@@ -2611,9 +2615,12 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
     for (; hasTemplateArgumentForDeduction(As, ArgIdx) &&
            PackScope.hasNextElement();
          ++ArgIdx) {
+      TemplateArgument Pi = Pattern, Ai = As[ArgIdx];
+      if (Swapped)
+        std::swap(Pi, Ai);
       // Deduce template arguments from the pattern.
-      if (auto Result = DeduceTemplateArguments(S, TemplateParams, Pattern,
-                                                As[ArgIdx], Info, Deduced);
+      if (auto Result =
+              DeduceTemplateArguments(S, TemplateParams, Pi, Ai, Info, Deduced);
           Result != TemplateDeductionResult::Success)
         return Result;
 
@@ -2636,7 +2643,8 @@ TemplateDeductionResult Sema::DeduceTemplateArguments(
     SmallVectorImpl<DeducedTemplateArgument> &Deduced,
     bool NumberOfArgumentsMustMatch) {
   return ::DeduceTemplateArguments(*this, TemplateParams, Ps, As, Info, Deduced,
-                                   NumberOfArgumentsMustMatch);
+                                   NumberOfArgumentsMustMatch,
+                                   /*Swapped=*/false);
 }
 
 /// Determine whether two template arguments are the same.
@@ -3272,7 +3280,7 @@ DeduceTemplateArguments(Sema &S, T *Partial,
   if (TemplateDeductionResult Result = ::DeduceTemplateArguments(
           S, Partial->getTemplateParameters(),
           Partial->getTemplateArgs().asArray(), TemplateArgs, Info, Deduced,
-          /*NumberOfArgumentsMustMatch=*/false);
+          /*NumberOfArgumentsMustMatch=*/false, /*Swapped=*/false);
       Result != TemplateDeductionResult::Success)
     return Result;
 
@@ -6187,7 +6195,8 @@ bool Sema::isMoreSpecializedThanPrimary(
 }
 
 bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
-     TemplateParameterList *P, TemplateDecl *AArg, SourceLocation Loc) {
+    TemplateParameterList *P, TemplateDecl *AArg, SourceLocation Loc,
+    bool IsDeduced) {
   // C++1z [temp.arg.template]p4: (DR 150)
   //   A template template-parameter P is at least as specialized as a
   //   template template-argument A if, given the following rewrite to two
@@ -6197,11 +6206,10 @@ bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
   // equivalent partial ordering by performing deduction directly on
   // the template parameter lists of the template template parameters.
   //
-  //   Given an invented class template X with the template parameter list of
-  //   A (including default arguments):
-  TemplateName X = Context.getCanonicalTemplateName(TemplateName(AArg));
   TemplateParameterList *A = AArg->getTemplateParameters();
 
+  //   Given an invented class template X with the template parameter list of
+  //   A (including default arguments):
   //    - Each function template has a single function parameter whose type is
   //      a specialization of X with template arguments corresponding to the
   //      template parameters from the respective function template
@@ -6242,14 +6250,45 @@ bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
       return false;
   }
 
-  QualType AType = Context.getCanonicalTemplateSpecializationType(X, AArgs);
-  QualType PType = Context.getCanonicalTemplateSpecializationType(X, PArgs);
+  // Determine whether P1 is at least as specialized as P2.
+  TemplateDeductionInfo Info(Loc, A->getDepth());
+  SmallVector<DeducedTemplateArgument, 4> Deduced;
+  Deduced.resize(A->size());
 
   //   ... the function template corresponding to P is at least as specialized
   //   as the function template corresponding to A according to the partial
   //   ordering rules for function templates.
-  TemplateDeductionInfo Info(Loc, A->getDepth());
-  return isAtLeastAsSpecializedAs(*this, PType, AType, AArg, Info);
+
+  // Provisional resolution for CWG2398: Regarding temp.arg.template]p4, when
+  // applying the partial ordering rules for function templates on
+  // the rewritten template template parameters:
+  //   - In a deduced context, the specific rules in [temp.deduct.type]p9
+  //   regarding how the argument lists themselves are matched is swapped
+  //   between P and A.
+  //   Note: The roles of the elements themselves are not swapped.
+  ArrayRef<TemplateArgument> Ps = AArgs, As = PArgs;
+  if (IsDeduced)
+    std::swap(Ps, As);
+  if (::DeduceTemplateArguments(*this, A, Ps, As, Info, Deduced,
+                                /*NumberOfArgumentsMustMatch=*/false,
+                                /*Swapped=*/IsDeduced) !=
+      TemplateDeductionResult::Success)
+    return false;
+
+  SmallVector<TemplateArgument, 4> DeducedArgs(Deduced.begin(), Deduced.end());
+  Sema::InstantiatingTemplate Inst(*this, Info.getLocation(), AArg, DeducedArgs,
+                                   Info);
+  if (Inst.isInvalid())
+    return false;
+
+  bool AtLeastAsSpecialized;
+  runWithSufficientStackSpace(Info.getLocation(), [&] {
+    AtLeastAsSpecialized =
+        ::FinishTemplateArgumentDeduction(
+            *this, AArg, /*IsPartialOrdering=*/true, PArgs, Deduced, Info) ==
+        TemplateDeductionResult::Success;
+  });
+  return AtLeastAsSpecialized;
 }
 
 namespace {
diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp
index a20155486b123d..9f1c6baa0a5a1f 100644
--- a/clang/test/SemaTemplate/cwg2398.cpp
+++ b/clang/test/SemaTemplate/cwg2398.cpp
@@ -62,25 +62,19 @@ namespace templ {
 namespace type_pack1 {
   template<class T2> struct A;
   template<template<class ...T3s> class TT1, class T4> struct A<TT1<T4>>   ;
-  // new-note@-1 {{partial specialization matches}}
   template<template<class    T5 > class TT2, class T6> struct A<TT2<T6>> {};
-  // new-note@-1 {{partial specialization matches}}
 
   template<class T1> struct B;
   template struct A<B<char>>;
-  // new-error@-1 {{ambiguous partial specialization}}
 } // namespace type_pack1
 
 namespace type_pack2 {
   template<class T2> struct A;
   template<template<class ...T3s> class TT1, class ...T4> struct A<TT1<T4...>>   ;
-  // new-note@-1 {{partial specialization matches}}
   template<template<class    T5 > class TT2, class ...T6> struct A<TT2<T6...>> {};
-  // new-note@-1 {{partial specialization matches}}
 
   template<class T1> struct B;
   template struct A<B<char>>;
-  // new-error@-1 {{ambiguous partial specialization}}
 } // namespace type_pack2
 
 namespace type_pack3 {

Endilll

Sema.h changes look good to me.

h-vetinari

Despite CWG2398 not being voted in yet, the status in

llvm-project/clang/www/cxx_dr_status.html

Lines 14195 to 14200 in e3f42b0

    
           <tr class="open" id="2398"> 
        
             <td><a href="https://cplusplus.github.io/CWG/issues/2398.html">2398</a></td> 
        
             <td>drafting</td> 
        
             <td>Template template parameter matching and deduction</td> 
        
             <td align="center">Not resolved</td> 
        
           </tr>

should be updated.

clang/test/SemaTemplate/cwg2398.cpp

clang/lib/Sema/SemaTemplateDeduction.cpp

This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. When performing template argument deduction, a template template parameter containing no packs should be more specialized than one that does. Given the following example: ```C++ template<class T2> struct A; template<template<class ...T3s> class TT1, class T4> struct A<TT1<T4>>; // #1 template<template<class T5 > class TT2, class T6> struct A<TT2<T6>>; // #2 template<class T1> struct B; template struct A<B<char>>; ``` Prior to P0522, candidate #2 would be more specialized. After P0522, neither is more specialized, so this becomes ambiguous. With this change, #2 becomes more specialized again, maintaining compatibility with pre-P0522 implementations. The problem is that in P0522, candidates are at least as specialized when matching packs to fixed-size lists both ways, whereas before, a fixed-size list is more specialized. This patch keeps the original behavior when checking template arguments outside deduction, but restores this aspect of pre-P0522 matching during deduction. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.

Endilll · 2024-05-16T12:47:39Z

Despite CWG2398 not being voted in yet, the status in

llvm-project/clang/www/cxx_dr_status.html

Lines 14195 to 14200 in e3f42b0

<tr class="open" id="2398">

<td><a href="https://cplusplus.github.io/CWG/issues/2398.html">2398</a></td>

<td>drafting</td>

<td>Template template parameter matching and deduction</td>

<td align="center">Not resolved</td>

</tr>

should be updated.

The only way to update it is to add a test to clang/test/CXX/drs. Me and the author had a discussion couple of weeks ago, and the conclusion was that what he implements is not (yet) documented in the CWG2398 filing, so we don't have anything to write a DR test and claim conformance against. If something has changes since then, we should revisit that conclusion.

cor3ntin

LGTM but gives @zygoloid @erichkeane a few days to review

zygoloid

I think we should go ahead with this. The behavior here is subtle but I think it does make sense, and we're in the process of proposing this change to WG21.

joker-eph · 2024-05-16T22:28:05Z

Seems like a bot is broken: https://lab.llvm.org/buildbot/#/builders/271/builds/7701 ; can you check?

mizvekov · 2024-05-16T22:40:30Z

That test was merged after the last time pre-commit CI was run on this MR.

The change looks like a consequence of my refactoring, we now preserve the type sugar from the injected arguments.

joker-eph · 2024-05-16T22:44:03Z

So are we reverting here or do you have quick fix available?

mizvekov · 2024-05-16T22:44:30Z

The quick fix would be to change the expectations of the test, I can do it for you.

mizvekov · 2024-05-16T22:46:44Z

Weirdly enough the test passes on my machine, latest MacOS.

Maybe the test is not constrained on target, and this is causing differences between machines?

mizvekov · 2024-05-16T22:50:46Z

Yep, I confirm the behavior happens if I add -triple x86_64-windows-msvc to RUN line.

mizvekov · 2024-05-16T22:57:22Z

I just pushed a fix.

joker-eph · 2024-05-16T23:00:49Z

Great, thanks for the quick fix!

mizvekov · 2024-05-17T00:25:14Z

I just double checked, the issue is present on main before this PR was merged, it's completely unrelated.

joker-eph · 2024-05-17T04:39:10Z

You're right, it's visible on the link I posted, the build was already broken! Somehow I fat-fingered and didn't hit the first red build but the third one!

mizvekov requested review from zygoloid, cor3ntin, AaronBallman and erichkeane May 2, 2024 05:11

mizvekov self-assigned this May 2, 2024

mizvekov requested a review from Endilll as a code owner May 2, 2024 05:11

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 2, 2024

Endilll reviewed May 2, 2024

View reviewed changes

h-vetinari reviewed May 2, 2024

View reviewed changes

clang/test/SemaTemplate/cwg2398.cpp Outdated Show resolved Hide resolved

mizvekov mentioned this pull request May 7, 2024

[clang] Enable C++17 relaxed template template argument matching by default #89807

Merged

cor3ntin reviewed May 8, 2024

View reviewed changes

clang/lib/Sema/SemaTemplateDeduction.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaTemplateDeduction.cpp Outdated Show resolved Hide resolved

mizvekov force-pushed the users/mizvekov/clang-cwg2398-disambiguate-packs branch from b7df9a3 to c4b72af Compare May 16, 2024 00:50

mizvekov force-pushed the users/mizvekov/clang-cwg2398-disambiguate-packs branch from c4b72af to 39e0af9 Compare May 16, 2024 05:51

cor3ntin approved these changes May 16, 2024

View reviewed changes

erichkeane approved these changes May 16, 2024

View reviewed changes

zygoloid reviewed May 16, 2024

View reviewed changes

mizvekov merged commit c86a53d into main May 16, 2024
3 of 4 checks passed

mizvekov deleted the users/mizvekov/clang-cwg2398-disambiguate-packs branch May 16, 2024 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang] Implement provisional wording for CWG2398 regarding packs #90820

[clang] Implement provisional wording for CWG2398 regarding packs #90820

mizvekov commented May 2, 2024 •

edited

Loading

llvmbot commented May 2, 2024

Endilll left a comment

h-vetinari left a comment

Endilll commented May 16, 2024

cor3ntin left a comment

zygoloid left a comment

joker-eph commented May 16, 2024

mizvekov commented May 16, 2024

joker-eph commented May 16, 2024

mizvekov commented May 16, 2024

mizvekov commented May 16, 2024

mizvekov commented May 16, 2024

mizvekov commented May 16, 2024

joker-eph commented May 16, 2024

mizvekov commented May 17, 2024

joker-eph commented May 17, 2024

	<tr class="open" id="2398">
	<td><a href="https://cplusplus.github.io/CWG/issues/2398.html">2398</a></td>
	<td>drafting</td>
	<td>Template template parameter matching and deduction</td>
	<td align="center">Not resolved</td>
	</tr>

[clang] Implement provisional wording for CWG2398 regarding packs #90820

[clang] Implement provisional wording for CWG2398 regarding packs #90820

Conversation

mizvekov commented May 2, 2024 • edited Loading

llvmbot commented May 2, 2024

Endilll left a comment

Choose a reason for hiding this comment

h-vetinari left a comment

Choose a reason for hiding this comment

Endilll commented May 16, 2024

cor3ntin left a comment

Choose a reason for hiding this comment

zygoloid left a comment

Choose a reason for hiding this comment

joker-eph commented May 16, 2024

mizvekov commented May 16, 2024

joker-eph commented May 16, 2024

mizvekov commented May 16, 2024

mizvekov commented May 16, 2024

mizvekov commented May 16, 2024

mizvekov commented May 16, 2024

joker-eph commented May 16, 2024

mizvekov commented May 17, 2024

joker-eph commented May 17, 2024

mizvekov commented May 2, 2024 •

edited

Loading